make -C libpq check fails obscurely if tap tests are disabled

  • Jump to comment-1
    pryzby@telsasoft.com2022-07-20T17:23:22+00:00
    make -C ./src/interfaces/libpq check PATH=... && @echo "TAP tests not enabled. Try configuring with --enable-tap-tests" /bin/sh: 1: @echo: not found make is telling the shell to run "@echo" , rather than running "echo" silently. Since: commit 6b04abdfc5e0653542ac5d586e639185a8c61a39 Author: Andres Freund <andres@anarazel.de> Date: Sat Feb 26 16:51:47 2022 -0800 Run tap tests in src/interfaces/libpq. -- Justin
    • Jump to comment-1
      andrew@dunslane.net2022-07-20T19:00:28+00:00
      On 2022-07-20 We 13:23, Justin Pryzby wrote: > make -C ./src/interfaces/libpq check > PATH=... && @echo "TAP tests not enabled. Try configuring with --enable-tap-tests" > /bin/sh: 1: @echo: not found > > make is telling the shell to run "@echo" , rather than running "echo" silently. > > Since: > > commit 6b04abdfc5e0653542ac5d586e639185a8c61a39 > Author: Andres Freund <andres@anarazel.de> > Date: Sat Feb 26 16:51:47 2022 -0800 > > Run tap tests in src/interfaces/libpq. Yeah. It's a bit ugly but I think the attached would fix it. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
      • Jump to comment-1
        alvherre@alvh.no-ip.org2022-07-21T08:53:48+00:00
        On 2022-Jul-20, Andrew Dunstan wrote: > On 2022-07-20 We 13:23, Justin Pryzby wrote: > > PATH=... && @echo "TAP tests not enabled. Try configuring with --enable-tap-tests" > > /bin/sh: 1: @echo: not found > > > > make is telling the shell to run "@echo" , rather than running "echo" silently. > Yeah. It's a bit ugly but I think the attached would fix it. Here's a different take. Just assign the variable separately. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "No renuncies a nada. No te aferres a nada."
        • Jump to comment-1
          andrew@dunslane.net2022-07-21T20:00:17+00:00
          On 2022-07-21 Th 04:53, Alvaro Herrera wrote: > On 2022-Jul-20, Andrew Dunstan wrote: > >> On 2022-07-20 We 13:23, Justin Pryzby wrote: >>> PATH=... && @echo "TAP tests not enabled. Try configuring with --enable-tap-tests" >>> /bin/sh: 1: @echo: not found >>> >>> make is telling the shell to run "@echo" , rather than running "echo" silently. >> Yeah. It's a bit ugly but I think the attached would fix it. > Here's a different take. Just assign the variable separately. Nice, I didn't know you could do that. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
          • Jump to comment-1
            alvherre@alvh.no-ip.org2022-07-22T18:23:23+00:00
            On 2022-Jul-21, Andrew Dunstan wrote: > On 2022-07-21 Th 04:53, Alvaro Herrera wrote: > > Here's a different take. Just assign the variable separately. > > Nice, I didn't know you could do that. It's not very common -- we do have some target-specific variable assignments, but none of them use 'export'. I saw somewhere that this works from Make 3.77 onwards, and we require 3.80, so it should be okay. The buildfarm will tell us ... -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "Industry suffers from the managerial dogma that for the sake of stability and continuity, the company should be independent of the competence of individual employees." (E. Dijkstra)
            • Jump to comment-1
              alvherre@alvh.no-ip.org2022-07-22T20:19:45+00:00
              On 2022-Jul-22, Alvaro Herrera wrote: > It's not very common -- we do have some target-specific variable > assignments, but none of them use 'export'. I saw somewhere that this > works from Make 3.77 onwards, and we require 3.80, so it should be okay. > The buildfarm will tell us ... Hm, so prairiedog didn't like this: make -C libpq all Makefile:146: *** multiple target patterns. Stop. but I don't understand which part it is upset about. The rules are: check installcheck: export PATH := $(CURDIR)/test:$(PATH) check: test-build all $(prove_check) installcheck: test-build all $(prove_installcheck) I think "multiple target patterns" means it doesn't like the fact that there are two colons in the first line. But if I use a recursive assignment (PATH = ...), that of course doesn't work because PATH appears on both sides of the assignment: Makefile:146: *** Recursive variable 'PATH' references itself (eventually). Stop. Now, maybe that colon is not the issue and perhaps the problem can be solved by splitting the rule: check: export PATH := $(CURDIR)/test:$(PATH) installcheck: export PATH := $(CURDIR)/test:$(PATH) According to 32568.1536241083@sss.pgh.pa.us, prairiedog is on Make 3.80. Hmmm. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ Voy a acabar con todos los humanos / con los humanos yo acabaré voy a acabar con todos (bis) / con todos los humanos acabaré ¡acabaré! (Bender)
              • Jump to comment-1
                tgl@sss.pgh.pa.us2022-07-22T20:40:43+00:00
                Alvaro Herrera <alvherre@alvh.no-ip.org> writes: > On 2022-Jul-22, Alvaro Herrera wrote: >> It's not very common -- we do have some target-specific variable >> assignments, but none of them use 'export'. I saw somewhere that this >> works from Make 3.77 onwards, and we require 3.80, so it should be okay. >> The buildfarm will tell us ... > Hm, so prairiedog didn't like this: > According to 32568.1536241083@sss.pgh.pa.us, prairiedog is on Make 3.80. Yeah, it is. I looked at the gmake manual on that machine, and its description of "export" seems about the same as what I see in a modern version. So it should work ... but we've found bugs in 3.80 before. Let me poke at it and see if there's a variant that works. The wording of the message suggests that maybe breaking it into two separate rules would help. regards, tom lane
                • Jump to comment-1
                  tgl@sss.pgh.pa.us2022-07-22T21:24:11+00:00
                  I wrote: > Yeah, it is. I looked at the gmake manual on that machine, and its > description of "export" seems about the same as what I see in a > modern version. Um ... I was not looking in the right place. The description of "target-specific variables" does not say you can use "export", whereas the modern manual mentions that specifically. I found a relevant entry in their changelog: 2002-10-13 Paul D. Smith <psmith@gnu.org> ... * read.c (eval): Fix Bug #1391: allow "export" keyword in target-specific variable definitions. Check for it and set an "exported" flag. * doc/make.texi (Target-specific): Document the ability to use "export". So it'll work in 3.81 (released 2006) and later, but not 3.80. TBH my inclination here is to move our goalposts to say "we support gmake 3.81 and later". It's possible that prairiedog's copy of 3.80 is the last one left in the wild, and nearly certain that it's the last one left that anyone would try to build PG with. (I see gmake 3.81 in the next macOS version, 10.5.) I doubt it'd take long to install a newer version on prairiedog. Alternatively, we could use Andrew's hacky solution from upthread. regards, tom lane
                  • Jump to comment-1
                    tgl@sss.pgh.pa.us2022-07-22T23:50:02+00:00
                    I wrote: > So it'll work in 3.81 (released 2006) and later, but not 3.80. Confirmed that things are fine with 3.81. > TBH my inclination here is to move our goalposts to say "we support > gmake 3.81 and later". Barring objections, I'll push the attached patch. I suppose we could undo whatever dumbing-down was done in _create_recursive_target, but is it worth troubling with? regards, tom lane
                    • Jump to comment-1
                      andres@anarazel.de2022-07-26T11:28:21+00:00
                      Hi, On 2022-07-22 19:50:02 -0400, Tom Lane wrote: > I wrote: > > So it'll work in 3.81 (released 2006) and later, but not 3.80. > > Confirmed that things are fine with 3.81. Thanks for looking into this Alvaro, Andrew, Justin, Tom - I was on vacation... Greetings, Andres Freund
                    • Jump to comment-1
                      alvherre@alvh.no-ip.org2022-07-25T10:32:55+00:00
                      On 2022-Jul-22, Tom Lane wrote: > Barring objections, I'll push the attached patch. I suppose we > could undo whatever dumbing-down was done in _create_recursive_target, > but is it worth troubling with? Excellent, many thanks. I tried to get Make 3.80 built here, to no avail. I have updated that to 3.81, but still haven't found a way past the automake phase. Anyway, I tried a revert of 1bd201214965 -- I ended up with the attached. However, while a serial compile fails, parallel ones fail randomly, and apparently because two submakes compete in building libpq.a and each deletes the other's file. What I think this is saying is that the 3.80-induced wording of that function limits concurrency of the generated recursive rules, which prevents the problem from occurring; and if we were to fix that bug we would probably end up with more concurrency. Here's the bottom of the 'make -j8' log: rm -f libpq.a ranlib libpq.a ranlib: 'libpq.a': No such file make[5]: *** [/pgsql/source/master/src/Makefile.shlib:261: libpq.a] Error 1 make[5]: *** Waiting for unfinished jobs.... ar crs libpq.a fe-auth-scram.o fe-connect.o fe-exec.o fe-lobj.o fe-misc.o fe-print.o fe-protocol3.o fe-secure.o fe-trace.o legacy-pqsignal.o libpq-events.o pqexpbuffer.o fe-auth.o fe-secure-common.o fe-secure-openssl.o ranlib libpq.a touch libpq.a rm -f libpq.so.5 ln -s libpq.so.5.16 libpq.so.5 rm -f libpq.so ln -s libpq.so.5.16 libpq.so touch libpq-refs-stamp rm -f libpq.so.5 ln -s libpq.so.5.16 libpq.so.5 rm -f libpq.so ln -s libpq.so.5.16 libpq.so rm -f libpq.so.5 ln -s libpq.so.5.16 libpq.so.5 touch libpq-refs-stamp rm -f libpq.so gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -pthread -D_REENTRANT -D_THREAD_SAFE -fPIC -shared -Wl,-soname,libecpg.so.6 -Wl,--version-script=exports.list -o libecpg.so.6.16 connect.o data.o descriptor.o error.o execute.o memory.o misc.o prepare.o sqlda.o typename.o -L../../../../src/port -L../../../../src/common -L../pgtypeslib -lpgtypes -L../../../../src/common -lpgcommon_shlib -L../../../../src/port -lpgport_shlib -L../../../../src/interfaces/libpq -lpq -L/usr/lib/llvm-11/lib -Wl,--as-needed -Wl,-rpath,'/pgsql/install/master/lib',--enable-new-dtags -lm ln -s libpq.so.5.16 libpq.so rm -f libecpg.a make[4]: *** [../../../../src/Makefile.global:618: submake-libpq] Error 2 ar crs libecpg.a connect.o data.o descriptor.o error.o execute.o memory.o misc.o prepare.o sqlda.o typename.o make[3]: *** [Makefile:17: all-ecpglib-recursive] Error 2 make[3]: *** Waiting for unfinished jobs.... ranlib libecpg.a touch libecpg.a rm -f libecpg.so.6 ln -s libecpg.so.6.16 libecpg.so.6 rm -f libecpg.so ln -s libecpg.so.6.16 libecpg.so gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -pthread -D_REENTRANT -D_THREAD_SAFE -fPIC -shared -Wl,-soname,libecpg_compat.so.3 -Wl,--version-script=exports.list -o libecpg_compat.so.3.16 informix.o -L../../../../src/port -L../../../../src/common -L../ecpglib -lecpg -L../pgtypeslib -lpgtypes -L../../../../src/common -lpgcommon_shlib -L../../../../src/port -lpgport_shlib -L../../../../src/interfaces/libpq -lpq -L/usr/lib/llvm-11/lib -Wl,--as-needed -Wl,-rpath,'/pgsql/install/master/lib',--enable-new-dtags -lm rm -f libecpg_compat.a ar crs libecpg_compat.a informix.o ranlib libecpg_compat.a touch libecpg_compat.a rm -f libecpg_compat.so.3 ln -s libecpg_compat.so.3.16 libecpg_compat.so.3 rm -f libecpg_compat.so ln -s libecpg_compat.so.3.16 libecpg_compat.so make[2]: *** [Makefile:17: all-ecpg-recursive] Error 2 make[1]: *** [Makefile:42: all-interfaces-recursive] Error 2 make: *** [GNUmakefile:11: all-src-recursive] Error 2 -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Los dioses no protegen a los insensatos. Éstos reciben protección de otros insensatos mejor dotados" (Luis Wu, Mundo Anillo)
                      • Jump to comment-1
                        alvherre@alvh.no-ip.org2022-07-25T10:33:39+00:00
                        On 2022-Jul-25, Alvaro Herrera wrote: > Anyway, I tried a revert of 1bd201214965 -- I ended up with the > attached. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
                        • Jump to comment-1
                          alvherre@alvh.no-ip.org2022-07-26T08:21:24+00:00
                          Ah, I found what the actual problem is: we have sprinkled a few dependencies on "...-recurse" throught the tree, but the patch I posted yesterday changes the manufactured target as "-recursive", as it was prior to 1bd201214965; so essentially these manually added dependencies all became silent no-ops. With this version I keep the target name as -recurse, and at least the ecpg<->libpq problem is no more AFAICT. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
                          • Jump to comment-1
                            alvherre@alvh.no-ip.org2022-07-26T15:40:19+00:00
                            On 2022-Jul-26, Alvaro Herrera wrote: > With this version I keep the target name as -recurse, and at least the > ecpg<->libpq problem is no more AFAICT. ... but I think we're missing some more dependencies, because if I remove everything (beyond make clean), then a "make -j8 world-bin" fails, per Cirrus. https://cirrus-ci.com/build/6305635338813440 With that, I'm going to set this aside for the time being. If somebody wants to play with these Makefile rules, be my guest. It sounds like there's some compile time gains to be had, but it may require some fiddling and it's not clear to me if the move to Meson is going to make this moot. Running it locally, I get this: [...] gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -DFRONTEND -I. -I/pgsql/source/master/src/common -I../../src/include -I/pgsql/source/master/src/include -D_GNU_SOURCE -DVAL_CC="\"gcc\"" -DVAL_CPPFLAGS="\"-D_GNU_SOURCE\"" -DVAL_CFLAGS="\"-Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2\"" -DVAL_CFLAGS_SL="\"-fPIC\"" -DVAL_LDFLAGS="\"-L/usr/lib/llvm-11/lib -Wl,--as-needed -Wl,-rpath,'/pgsql/install/master/lib',--enable-new-dtags\"" -DVAL_LDFLAGS_EX="\"\"" -DVAL_LDFLAGS_SL="\"\"" -DVAL_LIBS="\"-lpgcommon -lpgport -llz4 -lssl -lcrypto -lz -lreadline -lpthread -lrt -ldl -lm \"" -c -o hashfn.o /pgsql/source/master/src/common/hashfn.c -MMD -MP -MF .deps/hashfn.Po [...] gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -DFRONTEND -I. -I/pgsql/source/master/src/common -I../../src/include -I/pgsql/source/master/src/include -D_GNU_SOURCE -DVAL_CC="\"gcc\"" -DVAL_CPPFLAGS="\"-D_GNU_SOURCE\"" -DVAL_CFLAGS="\"-Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2\"" -DVAL_CFLAGS_SL="\"-fPIC\"" -DVAL_LDFLAGS="\"-L/usr/lib/llvm-11/lib -Wl,--as-needed -Wl,-rpath,'/pgsql/install/master/lib',--enable-new-dtags\"" -DVAL_LDFLAGS_EX="\"\"" -DVAL_LDFLAGS_SL="\"\"" -DVAL_LIBS="\"-lpgcommon -lpgport -llz4 -lssl -lcrypto -lz -lreadline -lpthread -lrt -ldl -lm \"" -c -o relpath.o /pgsql/source/master/src/common/relpath.c -MMD -MP -MF .deps/relpath.Po [...] In file included from /pgsql/source/master/src/include/postgres.h:47, from /pgsql/source/master/src/common/hashfn.c:24: /pgsql/source/master/src/include/utils/elog.h:75:10: fatal error: utils/errcodes.h: No existe el fichero o el directorio 75 | #include "utils/errcodes.h" | ^~~~~~~~~~~~~~~~~~ compilation terminated. [...] make[2]: *** [../../src/Makefile.global:945: hashfn.o] Error 1 make[2]: *** Se espera a que terminen otras tareas.... /pgsql/source/master/src/common/relpath.c:21:10: fatal error: catalog/pg_tablespace_d.h: No existe el fichero o el directorio 21 | #include "catalog/pg_tablespace_d.h" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[2]: *** [../../src/Makefile.global:945: relpath.o] Error 1 make[2]: se sale del directorio '/home/alvherre/Code/pgsql-build/master/src/common' make[1]: *** [Makefile:42: all-common-recurse] Error 2 make[1]: se sale del directorio '/home/alvherre/Code/pgsql-build/master/src' make: *** [GNUmakefile:21: world-bin-src-recurse] Error 2 -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "I must say, I am absolutely impressed with what pgsql's implementation of VALUES allows me to do. It's kind of ridiculous how much "work" goes away in my code. Too bad I can't do this at work (Oracle 8/9)." (Tom Allison) http://archives.postgresql.org/pgsql-general/2007-06/msg00016.php